closest-observation Sentinel-2 ingest builder (issue #509) - #511

🤖 from Claude (review)

A cover word is a run of buckets, not one bucket — so this midpoint collapses whole campaigns to a single epoch, and the ±4.9 h claim on line 49 is false.

quantize_words widens each instant to an aligned order-18 bucket and then canonicalizes with mortie.toc_normalize, and §10.5 spells out that normalize "coalesces ranges that merely abut" (src/zagg/coverage_toc.py:505-509 — that is exactly why the gap-survival floor is two bucket spans, not one). So the words that come out of a cover are maximal runs of contiguous buckets. Taking the midpoint of the word therefore yields one epoch per run, not one per pass, and its error is half the run, not half a bucket.

Measured against this commit (uv run python, order-18 pin):

# two passes 9.77 h apart (adjacent buckets)
inst = np.array([BASE, BASE + 2**45], dtype=np.uint64)
quantize_words(time2toc(inst))            # -> 1 word
_word_midpoints(...)                      # -> 1 epoch, 5.8 h / 3.9 h off the two passes

# a polar-style shard: one pass every 6 h for 30 days
inst = np.array([BASE + i*6*3600*10**9 for i in range(120)], dtype=np.uint64)
quantize_words(time2toc(inst))            # -> 1 word spanning 30.1 days
_word_midpoints(...)                      # -> 1 epoch, up to ±15 DAYS off

That is not a rounding nit for this builder: the issue's ruling budgets ±4.9 h against Sentinel-2's ~4.3-day revisit precisely so the nearest-acquisition pick in phase 2 is unambiguous. With run-midpoints, a dense shard (GEDI on a ~90 min ISS orbit, or any shard with repeat passes closer than ~19.5 h) contributes one epoch for a whole campaign and phase 2 will select one S2 granule for it. The ruling says the order-18 bucket midpoint names the pass epoch; the code takes the word midpoint, and after toc_normalize those are not the same thing.

Suggested fix — expand each word to its constituent pinned-order buckets before taking midpoints (the pin is already available: _object_pin / the block's temporal_order, see the separate comment below):

k = np.uint64(63 - pinned)
b0, b1 = start >> k, last >> k                 # first/last bucket index
buckets = np.unique(np.concatenate([np.arange(a, b + 1, dtype=np.uint64)
                                    for a, b in zip(b0, b1)]))
mid = (buckets << k) + np.uint64((1 << int(k)) // 2 - 1)

This restores one epoch per bucket, the ±4.9 h bound, and (as a bonus) makes the cross-store union canonical for free.

No test catches this because every fixture separation is chosen wide enough to never coalesce: _instants(0, 5, 11) in TestWordMidpoints and _instants(0, 5)/(5, 11) in TestReferenceEpochs are whole days apart (1 day = 2.46 buckets, so the buckets are never adjacent). Please add a case with two instants exactly one bucket apart and assert two epochs come back.

🤖 from Claude

Fixed in f70d60a. Confirmed the coalescing first — quantize_words(time2toc([BASE, BASE + 2**45])) really does return one word, so the word-envelope midpoint was naming one epoch per campaign.

_word_midpoints(words, order=TEMPORAL_COVER_ORDER) now expands each word into its constituent buckets before taking midpoints, vectorized (no Python loop over words):

b0 = (start >> np.uint64(k)).astype(np.int64)
b1 = (last >> np.uint64(k)).astype(np.int64)
counts = b1 - b0 + 1
offsets = np.cumsum(counts) - counts
within = np.arange(int(counts.sum()), dtype=np.int64) - np.repeat(offsets, counts)
buckets = np.unique(np.repeat(b0, counts) + within).astype(np.uint64)
half = np.uint64((1 << k) // 2 - 1)
mid = np.minimum((buckets << np.uint64(k)) + half, np.uint64(mortie.TOC_MAX_NS))

Bucket midpoints keep the module's uniform last-covered-instant rule — (b << k) + 2**(k-1) - 1, your formula — so the exact-timestamp convention is unchanged. The TOC_MAX_NS clamp is for the top bucket at order 31 (2**(k-1) > 2**32 there is false, so its raw midpoint would sit past the scale ceiling).

Module and function docstrings rewritten: a word is a maximal RUN of buckets, the epochs are per-bucket, and the ±4.9 h bound is stated per bucket. The straddle case is documented as benign over-selection rather than error — a pass whose envelope crosses a bucket edge yields two epochs, both within half a bucket of the pass, both selecting the same nearest acquisition, and the builder dedupes granule ids per shard.

New tests in TestWordMidpoints (all measured before pinning):

• test_two_passes_one_bucket_apart_yield_two_epochs — asserts the coalescing (len(words) == 1) and then 2 epochs;
• test_a_contiguous_campaign_yields_one_epoch_per_covered_bucket — 40 instants at 6 h over 10 days: 1 word, 25 covered buckets, 25 epochs, max nearest-gap 16.96 h/2 … i.e. ≤ half a bucket;
• test_a_pass_straddling_a_bucket_edge_yields_two_epochs;
• test_a_coarser_order_widens_the_buckets — the order argument drives the grid (feeds the per-block fix below);
• test_an_exact_timestamp_word_decodes_to_its_own_instant renamed to …_to_its_bucket_midpoint, since an unquantized word now resolves to the bucket it falls in (within half a bucket of the instant).

🤖 from Claude (review)

np.unique over concatenated raw words is not a canonical union — two stores can (and do) emit different words for the same pass, so the same pass yields two epochs.

The docstring at line 138 claims: "two stores quantized on the same order-18 grid yield the same word for the same pass window, so the union is exact, never doubled." That holds for the bucket grid, but the words a cover carries are post-toc_normalize runs of buckets, and how far a run extends depends on that store's other data. Two stores sharing one pass therefore emit overlapping-but-unequal range words, both of which survive np.unique.

Measured against this commit — three true passes, one bucket apart; store A saw passes 1+2, store B saw passes 2+3:

store A epochs : 2019-05-07T00:12:01.084337151        # midpoint of buckets {0,1}
store B epochs : 2019-05-07T09:58:25.456425983        # midpoint of buckets {1,2}
union epochs   : both of the above                    # 2 epochs for 3 passes
true passes    : 2019-05-06T20:26:22, 2019-05-07T06:12:46, 2019-05-07T15:59:10

Note the shared pass (06:12:46) is represented twice, by two different epochs, and neither union epoch lands within ±4.9 h of a real pass. That is the doubling the docstring says cannot happen, and phase 2 will spend two nearest-acquisition selections on it.

test_union_across_stores_is_deduplicated passes only because its instants (_instants(0, 5) vs _instants(5, 11)) are whole days apart, so both stores produce byte-identical single-bucket words.

Suggested fix: the bucket-expansion in the _word_midpoints comment above makes this exact by construction (union of bucket indices is canonical). If you'd rather keep word-level union, at minimum run the concatenation through mortie.toc_normalize (or re-quantize_words at the pin) before decoding, and please add a regression test with two stores whose covers overlap partially rather than exactly.

🤖 from Claude

Fixed in 1cec8ed (mechanism landed with the bucket expansion in f70d60a).

Reproduced your counterexample exactly — three passes one bucket apart, store A saw 1+2 and store B saw 2+3 — and confirmed the two stores emit unequal single words for the shared pass. With bucket-level union the same input now gives 3 epochs for 3 passes, max nearest-gap 1.13 h against a 4.89 h bound, and union == np.union1d(store A, store B) still holds.

The docstring claim is corrected rather than restated: it no longer says two stores yield the same word, it says the union is canonical at the bucket level and explains why word-level union is not —

Words are post-toc_normalize runs whose extent depends on that store's other data, so two stores sharing one pass routinely emit overlapping-but-unequal range words — a raw np.unique over words would keep both and represent the shared pass twice, at two displaced midpoints. Expanding to buckets first removes that degree of freedom: the bucket grid is fixed by the order alone, so a shared pass contributes the same bucket midpoint from every store and dedupes exactly.

Implementation-wise reference_epochs no longer unions words at all — it unions decoded midpoints per shard (np.unique(np.concatenate(mids_by_shard[shard]))), which is also what lets two stores at different effective temporal orders compose at all (see the per-block thread).

Regression test test_partially_overlapping_covers_union_at_the_bucket_grid pins your case, including the premise (len(wa) == len(wb) == 1 and wa[0] != wb[0]) so it stays a real counterexample rather than an accident of the fixture spacing.

🤖 from Claude (review)

cover_words() discards each block's temporal_order, so a cap-coarsened shard is accepted silently and its epochs are quietly far coarser than the pin this module's accuracy claim rests on.

§10.5 lets a block coarsen below the object's pin to fit COVER_CAP (_cap_cover, src/zagg/coverage_toc.py:545-556), and build_cover_section logs a warning and records the landed order in the block (temporal_order, _encode_cover_block:601). _decode_cover_block returns that order — but cover_words() throws it away (coverage_toc.py:1000, [0] of the tuple). This module consumes cover_words() and so never sees it.

Consequence, measured against this commit: a shard whose cover exceeds the 512-word cap coarsens to order 16 (2^47 ns ≈ 39 h buckets, so ±19.5 h) and reference_epochs returns those epochs with no warning, no record, and nothing on ReferenceEpochs a caller could inspect:

coverage[toc]: shard 11213 cover coarsened to temporal order 16 ...   # emitted at WRITE time only
reference_epochs(root) -> 473 epochs, offsets up to ~1.6 days from the true passes

Coarsening cascades (_cap_cover loops down to order 0), so a pathological shard can land at a bucket spanning the whole 2^63 ns scale and hand phase 2 a single meaningless epoch — while the module docstring still advertises ±4.9 h. This is precisely the surface phase 2's max_time_offset has to reason against: an epoch known to ±19.5 h cannot be gated against an S2 acquisition with an hours-scale tolerance.

Suggested fix: decode per block rather than through cover_words() — either use zagg.coverage_toc._decode_cover_block(decimal, block, _object_pin(cover)) (and, if reaching into privates is unwelcome, add a public cover_word_orders() / a with_orders=True flag beside cover_words in coverage_toc.py) — then:

1. logger.warning naming the shard, its landed order, and the resulting midpoint bound whenever order != TEMPORAL_COVER_ORDER — §10.5's posture for coarsening is "widening only, loudly recorded", and a read that silently drops the "loudly" half breaks it;
2. carry the effective per-shard order (or the derived midpoint tolerance) on ReferenceEpochs so phase 2's build report and max_time_offset can use it rather than assuming the pin.

There is no test for a coarsened block at all; a case that pushes a shard past COVER_CAP (562 quantized words does it) would pin whichever behavior you choose.

🤖 from Claude

Fixed in a6642f6 — both the coarsening blindness and the unposted cover.get("order") minor from the same function.

Kept it inside closest_obs.py (no new public accessor, and coverage_toc.py is at 1165 lines against the 1200 cap): cover_words(obj) still does the strict decode — count MUST-check, object-pin validation, the "never above the pin" refusal — and the per-block order is read straight back off the §10.5 grammar beside it:

pinned = int(cover.get("temporal_order", TEMPORAL_COVER_ORDER))
blocks = cover.get("shards") or {}
...
effective = int(blocks.get(decimal, {}).get("temporal_order", pinned))

Then, per your two points:

1. logger.warning whenever effective < TEMPORAL_COVER_ORDER, naming the store, the shard decimal, the landed order, the bucket span (2^{63-effective} ns) and the resulting bound (±2^{62-effective} ns, "not ±4.9 h"). The read half of §10.5's "widening only, loudly recorded".
2. ReferenceEpochs.orders: dict[int, int] — shard key → the coarsest contributing order (min across stores, since two stores can land at different orders on the same shard), filtered to the shards that survive the AOI. Plus ReferenceEpochs.tolerance(shard) -> np.timedelta64, the half-bucket that phase 2's max_time_offset should gate against instead of assuming the pin.

Each block's words are expanded at its own order, which is what makes a mixed-order cross-store union well defined at all (midpoints unioned, not words — see the union thread).

On the minor: int(cover.get("order")) on a body without order raised a bare TypeError: int() argument must be..., naming neither the store nor the key. It now goes through _shard_order(cover, root), which raises the module's ValueError naming the store and the offending value, with a test (test_a_cover_without_a_shard_order_refuses_by_name).

New TestCoarsenedBlock builds the case through the real producer — 600 single-bucket claims two buckets apart, which trips COVER_CAP and lands _cap_cover at order 17:

• test_the_block_really_coarsened — the premise, from the written bytes;
• test_a_coarsened_block_warns_and_reports_its_order — caplog sees "temporal order 17 … below the pinned", out.orders == {SHARD_KEY: 17}, tolerance() reports 2^45 ns;
• test_the_epochs_are_the_coarse_buckets_midpoints — every epoch's internal ns is ≡ 2^(k-1)-1 mod 2^k at the coarse k and lands on the coarse grid's covered bucket set, and the nearest-gap bound widens to half a coarse bucket;
• test_an_uncoarsened_block_neither_warns_nor_hides_its_order — no warning, orders still reports the pin.

Left standing for a human call: whether a shard coarsened far below the pin should refuse outright rather than warn. §10.5's posture is widening-plus-loud, and phase 2 now has orders/tolerance() to gate on, so I did not add a refusal threshold on my own.

🤖 from Claude

Resolved per espg's tolerance-aware ruling (2026-08-24) — folded in ecca696b:

• With max_time_offset set: an epoch whose cover-bucket half-span exceeds the offset cannot be paired to the stated precision and is dropped loudly, per epoch, as its own ledger category — rows carrying the block's effective temporal_order + cover_half_span_ns, counted in epochs_dropped_low_resolution, inside the epochs_total == epochs_paired + epochs_dropped invariant, and visible in the estimate=True report.
• With no offset: one warning per build names the effective resolution (the coarsest block's half-span) and pairing proceeds — widening is lawful (§10.5) and the caller declared no precision bar.
• Boundary pinned: half-span exactly at the offset stays pairable (strictly-greater drops), the same permissive side the selection gate's exactly-at rule pins.
• Per-epoch, not per-shard: ReferenceEpochs now carries epoch_orders (row-aligned effective order per epoch, finest claim wins on a duplicated midpoint), so a shard mixing a pinned store's epochs with a coarsened store's drops only the coarse ones — the rationale of record being that flat-warn risks silently arbitrary pairings from a cap-degraded store while flat-refuse fails whole builds over blocks that may not intersect the AOI.

Tests: drop-with-category under a cap, estimate reporting, warn-once-and-pair without a cap, the exactly-at boundary both sides, and the mixed-order shard. ReferenceEpochs.orders/tolerance() remain as the per-shard headline surface.

🤖 from Claude (review)

The golden-fixture assertion is a tautology — it pins nothing.

The class docstring says the committed fixture "pins the frozen grammar bytes", but line 182 recomputes the expected value with the same function under test:

epochs = out.epochs[SHARD_KEY]                       # == np.unique(_word_midpoints(words))
words  = cover_words(read_cover(...))[SHARD]
assert np.array_equal(epochs, np.unique(_word_midpoints(words)))

reference_epochs computes its result as exactly np.unique(_word_midpoints(np.unique(concat(words)))) over the one store's one shard (closest_obs.py:200-201), so this assertion is f(x) == f(x). Every defect in the midpoint math — including the word-vs-bucket collapse flagged on _word_midpoints — passes it unchanged. The only real check left is the ±1-year window on line 190, which a midpoint off by weeks still satisfies.

Suggested fix: pin the literal values. The fixture currently decodes to exactly two epochs:

assert np.array_equal(epochs, np.array(
    ["2019-05-14T03:14:07.595891711", "2019-05-19T00:31:00.060957695"],
    dtype="datetime64[ns]"))

(Those literals will move if the word-vs-bucket fix lands — regenerate them from the fixture once, by hand, rather than from the function.) Better still, if the fixture's generator (tools/generate_spec_fixtures.py) knows the synthetic instants it quantized, assert each epoch is within a half-bucket of a known instant — that is the property the ±4.9 h ruling actually claims, and it is the assertion a regression would have to break.

🤖 from Claude

Fixed in 970d7a0. Agreed — it was f(x) == f(x).

Pinned as literals, and they are the two values you quoted (the fixture's two clusters each fall inside a single order-18 bucket, so the word→bucket fix did not move them):

GOLDEN = np.array(
    ["2019-05-14T03:14:07.595891711", "2019-05-19T00:31:00.060957695"],
    dtype="datetime64[ns]",
)

Verified by hand against tests/data/spec/temporal rather than by re-running the function: the committed cover carries 2 words whose envelopes occupy buckets 151903 and 151915 — twelve apart, which is the fixture's TEMPORAL_GAP_DAYS = 5 at 9.77 h/bucket — and each pinned instant's internal ns is ≡ 2^44 - 1 (mod 2^45), i.e. an order-18 bucket midpoint under the module's last-covered-instant rule.

That check is now its own test rather than a comment (test_the_golden_epochs_are_order_18_bucket_midpoints): it asserts int(t) % 2**k == 2**(k-1) - 1 and int(t) >> k in (151903, 151915) on the literals, via mortie.from_datetime64 — arithmetic a regression in _word_midpoints cannot satisfy.

And I took the better suggestion too. The generator does record its synthetic instants: temporal.expected.json carries cells[*].obs_span_ns. test_every_fixture_observation_has_an_epoch_within_half_a_bucket loads those and asserts every true observation instant has an epoch within HALF_BUCKET — the property the ±4.9 h ruling actually claims, sourced from the fixture sidecar, not from anything this module computed.

test_the_golden_cover_decodes_to_the_pinned_epochs also now pins out.orders == {SHARD_KEY: TEMPORAL_COVER_ORDER}, so the fixture guards the uncoarsened path as well. The ±1-year window test stays as the cheap outer sanity check.

🤖 from Claude (review)

Duplicate acquisition times select opposite ends of the run depending on which flank the epoch is on — the left flank takes the LAST catalog record, the right flank the first. The docstring's "Equal acquisition times are broken by catalog record order (stable sort)" (line 346) is only half true, and the suite's own oracle encodes the other rule.

Probe (uv run python, worktree at 16777c4) — records 0, 1, 2 all stamped 2025-06-01T00:00, record 3 four days later:

epoch T-1h  -> sel [0]   # right flank: first record of the run
epoch T     -> sel [0]
epoch T+1h  -> sel [2]   # left flank: LAST record of the run

Mechanism: on the left flank nearest = pos - 1 lands on the end of the equal-time run in sorted space, and order[nearest] maps that to the highest catalog index; on the right flank pos is the run start, so it maps to the lowest.

Why it matters here specifically: duplicate acquisition times are the norm for the raster catalogs this pairs against — one Sentinel-2 datatake stamps many granules with the same acquisition instant — so this is a common path, not a tail. Two epochs straddling that instant then select two different rows for the same instant, which is exactly what phase 3's granule dedupe cannot collapse.

The suite does not catch it. test_matches_a_brute_force_oracle computes want = best[np.argmin(t[best])], i.e. the LOWEST catalog index on a tie in t — the opposite of what the left flank returns. It passes today only because 60 draws from a 400-day second-resolution grid never collide. Rerunning that same oracle with every acquisition time duplicated (times = concat(raw, raw), 20 unique instants, 30 epochs) gives 16 mismatches out of 30:

epoch 1  oracle want 9  got 29   equal-time group [9, 29]
epoch 4  oracle want 2  got 22   equal-time group [2, 22]
...
oracle mismatches: 16 of 30

Suggested fix — snap the chosen index to the start of its equal-time run, one line after nearest is computed:

nearest = np.searchsorted(ts, ts[nearest], side="left")

ts is sorted and argsort is stable, so this always resolves to the first catalog record of the run, on both flanks. Verified: re-running the duplicated-times oracle above with that line added gives mismatches: 0. Please also add duplicate acquisition times to test_matches_a_brute_force_oracle (or a dedicated case) so the rule is pinned — as written, the suite would accept either end.

🤖 from Claude

Fixed in e6efd13. Reproduced your probe first — three granules at 2025-06-01T00:00 plus one four days later gives sel [0] / [0] / [2] for the T-1h / T / T+1h epochs, and the duplicated-times oracle (times = concat(raw, raw), 20 instants, 30 epochs) reports 16 mismatches of 30.

Took your one-liner verbatim, right after nearest is computed:

# Equal acquisition times form one run in ``ts``; the left flank lands on
# its END and the right flank on its START, so snap to the run start —
# with a stable ``argsort`` that is the run's first catalog record, from
# either side.
nearest = np.searchsorted(ts, ts[nearest], side="left")

Re-measured after: the three epochs all give sel [0], and the duplicated-times oracle reports 0 mismatches of 30.

The docstring now states the rule the code implements rather than the half-truth — equal acquisition times (one Sentinel-2 datatake stamps many granules with the same instant) resolve to the FIRST record of the equal-time run, whichever flank the epoch approaches it from.

Tests: test_duplicate_acquisition_times_select_the_first_record pins your exact probe (all three approach directions → catalog index 0), and the oracle body is now a _check_oracle helper driven by two cases — the original draw plus test_matches_the_oracle_with_every_acquisition_time_duplicated, which is your duplicated-times run. The oracle expectation is unchanged (best[np.argmin(t[best])], i.e. lowest catalog index among the equidistant set), so it is the rule that is pinned, not the implementation.

🤖 from Claude (review)

NaT in either input silently produces a wrong pairing — two different mechanisms, both undocumented, both unguarded, and neither is what the Returns section promises ("NaT only when times is empty", line 339).

(1) NaT in times breaks the searchsorted precondition. np.argsort on datetime64 sorts NaT to the end, but times[order].astype("int64") turns it into iinfo(int64).min, so ts is not sorted in int64 space and np.searchsorted on line 362 is querying unsorted data. Probe:

t  = np.array(["2025-06-01T00", "NaT", "2025-06-01T10"], dtype="datetime64[ns]")
np.argsort(t, kind="stable")            # -> [0 2 1]  (NaT last)
nearest_acquisitions(np.array(["2025-06-01T01", "2025-06-01T09"], dtype="datetime64[ns]"), t)
# sel [0, 1]   off [-1h, 7474603636854775808]

The 09:00 epoch selects sel == 1, which is the NaT row — not the 10:00 acquisition one hour away. A single missing timestamp in a catalog page silently mis-pairs its neighbours.

(2) NaT in epochs overflows and pairs anyway. e becomes iinfo(int64).min, so ts[0] - e on line 367 wraps:

ep = np.array(["2025-06-01T01", "NaT"], dtype="datetime64[ns]")
nearest_acquisitions(ep, np.array(["2025-06-01T00", "2025-06-01T10"], dtype="datetime64[ns]"))
# sel [0, 0]   off [-1h, -7474636036854775808]

The NaT epoch selects a real acquisition with a garbage offset — no -1, no NaT, no warning. With a max_time_offset it happens to drop (sel -1) only because the garbage magnitude exceeds the cap, i.e. for the wrong reason, and the reported offset is still garbage — so the "loud record a drop rides" (lines 337-340) carries a nonsense near-miss distance.

Suggested fix: guard both up front, before the sort, and say so in the docstring. Either refuse loudly — which matches this module's house style (reference_epochs raises on debris rather than coping, lines 253-257) — or define the behaviour: drop NaT rows from times before argsort (remapping indices through the kept mask so selection still indexes the caller's array), and force NaT epochs to selection = -1, offsets = NaT. Whichever you pick, please add a test for each input; the suite currently exercises neither.

🤖 from Claude

Fixed in fc2a53e — refused loudly, the option matching this module's posture.

Both probes reproduced at 16777c4 before the change: the NaT-in-times case selected sel == 1 (the NaT row) for the 09:00 epoch with offset 7474603636854775808, and the NaT-in-epochs case selected a real acquisition with offset -7474636036854775808.

Guard is up front, before the sort, naming which input and how many:

for name, arr in (("epochs", epochs), ("times", times)):
    if np.isnat(arr).any():
        raise ValueError(f"{name} carries NaT ({int(np.isnat(arr).sum())} of {arr.size})")

Refusal rather than coping, for the reason you named: reference_epochs never emits NaT (bucket midpoints are constructed), and phase 3's catalog time parser refuses a granule with a missing acquisition time, so a NaT reaching here is caller debris — same treatment reference_epochs gives a malformed cover.

Docstring: both parameter entries say NaT refuses, and a new Raises section states the mechanism (sorts last but casts to iinfo(int64).min, so ts is left unsorted and the offset subtraction wraps). The Returns note keeps its "NaT only when times is empty" claim, which is now true — plus the saturation case from the overflow thread.

Tests: test_a_nat_acquisition_time_refuses and test_a_nat_epoch_refuses, each your probe input, matching on the input name in the message.

🤖 from Claude (review)

e - ts wraps silently for far-apart timestamps, and the wrapped distance can pass max_time_offset. The representable datetime64[ns] span is ~584 years (1677-09-21 .. 2262-04-11), but int64 nanoseconds only hold ~292 years of difference, so a pair more than ~292 y apart overflows on lines 366-367. numpy wraps int64 array arithmetic silently — no RuntimeWarning is raised.

Probe (uv run python, worktree at 16777c4):

t  = np.array(["1677-09-22T00:12:44"], dtype="datetime64[ns]")
ep = np.array(["2262-04-11T23:47:16"], dtype="datetime64[ns]")   # true gap: 584.5 years

cap=None       -> sel [0]  off [+1.000019 days]     # wrong sign AND wrong magnitude
cap=1 day      -> sel [-1] off [+1.000019 days]     # drops, but by a hair
cap=120 days   -> sel [0]  off [+1.000019 days]     # PAIRS an acquisition 584 years away

The failure mode is a silently wrong pairing that survives the cap gate, not an error — and the offsets report, which phase 3 is supposed to trust as the near-miss record, says "1 day".

The same int64 ceiling bites the cap conversion on line 352:

np.timedelta64(1000, "Y").astype("timedelta64[ns]").astype("int64")   # -> -5336536147419103232

so max_time_offset=np.timedelta64(1000, "Y") refuses with max_time_offset must be non-negative (got numpy.timedelta64(1000,'Y')) — a refusal naming the wrong problem.

Suggested fix: both flank distances are non-negative by construction (e >= ts[pos-1] and ts[pos] >= e), so computing the magnitudes in uint64 is exact for the whole representable span — 1.845e19 ns fits under 2^64, and two's-complement mod-2^64 subtraction gives the true difference even across the sign boundary. Verified:

a = np.array([hi], dtype="int64").view(np.uint64)   # 2262-04-11
b = np.array([lo], dtype="int64").view(np.uint64)   # 1677-09-22
int(a - b) == hi - lo   # True  (18446657672000000000)

Do the left/right comparison and the cap gate in uint64 magnitudes (cap likewise, after rejecting negatives), keep the sign from which flank won, and let offsets fall back to NaT for the rare difference that genuinely will not fit in timedelta64[ns] — a NaT offset is honest, +1 day is not. For the cap, converting via np.timedelta64(x, "ns") on an already-timedelta64 input still overflows, so an explicit overflow check on line 352 (or a check that the round-trip back to the input's unit is lossless) would replace the misleading "non-negative" message.

🤖 from Claude

Fixed in ad6a311. I took the uint64-magnitude route you verified, not the up-front span guard — it keeps the module honest over the whole representable range instead of refusing inputs that are merely far apart, and it cost four lines rather than a new precondition to document.

Reproduced first at 16777c4: 1677-09-22T00:12:44 vs 2262-04-11T23:47:16 gave off [+1.000019 days] for every cap, and cap=120 days paired the 584-year-away acquisition.

Flank distances are now unsigned magnitudes; sign comes from the winning flank:

tsu, eu = ts.view(np.uint64), e.view(np.uint64)
far = np.uint64(2**64 - 1)  # a missing flank, wider than any real span
left  = np.where(pos > 0,        eu - tsu[np.maximum(pos - 1, 0)],  far)
right = np.where(pos < ts.size,  tsu[np.minimum(pos, ts.size - 1)] - eu, far)
...
magnitude = np.where(take_right, right, left)
fits = magnitude <= np.uint64(np.iinfo(np.int64).max)
signed = np.minimum(magnitude, np.uint64(np.iinfo(np.int64).max)).astype(np.int64)
offsets = np.where(take_right, signed, -signed).astype("timedelta64[ns]")
offsets = np.where(fits, offsets, np.timedelta64("NaT"))
if cap is not None:
    selection = np.where(magnitude <= np.uint64(cap), selection, np.int64(-1))

Both flanks are non-negative by construction, so mod-2^64 subtraction of the int64 bit patterns is exact across the full 584-year span; far = 2^64 - 1 is strictly above the widest real gap (2^64 - 2), so a missing flank never wins by accident. The cap gate compares magnitudes, so an unrepresentable distance can no longer slip under a finite tolerance.

Measured after — your probe: cap=1 day and cap=120 days both give sel [-1] off [NaT]; cap=None gives sel [0] off [NaT] (the nearest is still the nearest, the report saturates). Saturation is documented in Returns rather than left implicit: NaT when times is empty, and for a difference past ~292 years, with the note that such an epoch selects nothing under any cap.

Cap conversion is checked before the sign test, so the message names the real problem:

max_time_offset does not convert exactly to nanoseconds (got numpy.timedelta64(1000,'Y'); timedelta64[ns] spans ~292 years)

via a lossless round-trip back to the input's own unit, plus an explicit NaT check ahead of it (must be a real duration) — that previously fell through to "must be non-negative" as well. A negative cap still gets the original message, and its test is unchanged.

Tests: test_a_gap_past_int64_nanoseconds_never_pairs_under_a_cap (your 1677-vs-2262 pair, both caps and uncapped), test_an_unconvertible_max_time_offset_refuses_by_name, test_a_nat_max_time_offset_refuses, and test_a_two_century_gap_still_reports_an_exact_offset — a 200 y / 150 y pair that fits, to pin that the uint64 path stays exact where the report can carry it.

🤖 from Claude (review)

{**spatial.metadata, ...} carries the spatial builds aoi_mask: Trueinto a map whoseaoi_maskpayload isNone— the exact misleadreproject guards against (shardmap.py:1738, meta.pop("aoi_mask", None), "The dropped per-shard AOI mask (aoi_mask=None below) must not still be advertised in the derived maps metadata").

Worse than a stale key: the strict-AOI mask is silently lost for closest-obs ingest runs. runner.py:2240 reads the manifests top-level aoi_masklist to build the per-cell lookup; with the list gone and the flag still advertised, a run configuredoutput.aoi_mask: true` writes unmasked cells and nothing says so.

Probe (worktree, order-4 grid with output.aoi_mask: true):

spatial aoi_mask meta: True payload len: 1
closest map metadata["aoi_mask"] = True
closest map .aoi_mask = None
round-trip aoi_mask: None meta says: True

Fix: the mask is per-shard and row-aligned to spatial.shard_keys, and the emitted shard set is a subset, so re-index it rather than drop it — collect spatial.aoi_mask[i] alongside each kept i in the join loop and pass that list as the ShardMap's fifth argument (keeping metadata["aoi_mask"] honest). If carrying it through is out of scope, then meta.pop("aoi_mask", None) like reproject does, so the manifest at least stops claiming a mask it does not have.

🤖 from Claude

Fixed in 96ca33b — took the fallback you named rather than the carry-through, and documented why.

meta.pop("aoi_mask", None) now runs right before the ShardMap(...) construction, with a comment citing shardmap.py:1738 as the precedent. The closest_obs_shardmap Notes gained a paragraph: the strict-AOI payload is the SPATIAL map's, is not carried onto the emitted map, and an output.aoi_mask ingest run must therefore compute its mask at run time rather than read one off this manifest.

New test test_the_spatial_aoi_mask_is_neither_carried_nor_claimed mirrors your probe (order-4 HealpixGrid with output.aoi_mask: true): it asserts the spatial build really does produce a payload + metadata["aoi_mask"] is True, then that the derived map has aoi_mask is None and no aoi_mask key in its metadata.

Why not the re-index: the mask semantics moved under 2d72afa (the aoi fix below). A parts/GeoJSON aoi now rides as the spatial build's region=, so _compute_aoi_mask is computed against the AOI ring for those runs and against the catalog bbox rectangle for an aoi=None run — two different meanings for the same carried payload, and neither is the closest-obs query's own AOI. Deciding which one a paired manifest should advertise is a design call, so the honest drop lands now and carrying it through stays standing for review.

🤖 from Claude (review)

A shard whose epochs find no spatially-assigned acquisitions continues without touching dropped or epochs_paired, so its epochs vanish from the ledger entirely. The espg ruling is "drop recorded loudly, never silent", and the docstring promises metadata["closest_obs"] records "the epoch totals, every dropped epoch with its near-miss offset" — but epochs_dropped reads 0 while epochs are being dropped.

Probe (cover claims two shards, the S2 catalog only reaches one):

epochs_total: 7 paired: 3 dropped: 0
shards_without_acquisitions: ["-52323"]
RECONCILE total - paired - dropped = 4

Four epochs are unaccounted. The warning names the shard, but the numbers an operator reconciles say nothing was dropped — and this is the largest drop class in practice (a catalog whose query bbox/time window undershoots the reference coverage), precisely the case that thins the product.

Fix: record them in the same ledger as the max_time_offset drops, e.g. in the i is None branch

dropped.extend(
    {"shard": decimal, "epoch": np.datetime_as_string(t), "nearest_offset_ns": None}
    for t in epoch_arr
)

so epochs_total == epochs_paired + epochs_dropped holds by construction. Add a two-shard test asserting that identity — the branch is currently untested: pytest --cov=zagg.catalog.closest_obs reports lines 575-576 and 611 (this continue and its warning) as the only uncovered builder lines, so every test in TestClosestObsShardmap runs on a single-shard fixture and the whole cross-shard join is unexercised.

🤖 from Claude

Fixed in ccb6b9b, essentially your patch.

The i is None branch now dropped.extend(...)s one row per epoch with nearest_offset_ns: None — the meaning the key already carried for a NaT near-miss, i.e. "no acquisition to measure against" — before the continue, so epochs_total == epochs_paired + epochs_dropped holds by construction. shards_without_acquisitions and its warning are unchanged; the dropped warning's parenthetical now reads (max_time_offset=..., or no acquisitions in the shard) so it stops naming a cap as the only cause. The Notes line describing dropped states the invariant and what a None offset means.

Your probe now reconciles:

total 7 paired 3 dropped 4 swa ['11212']

Tests: test_a_shard_the_catalog_never_reaches_ledgers_its_epochs is the two-shard scenario you asked for (cover claims 11213 + 11212, the S2 items only reach 11213) — it pins shards_without_acquisitions == ["11212"], epochs_paired == 3, the total == paired + dropped identity, that every 11212 row carries nearest_offset_ns is None, and the warning. The identity assertion also went into test_max_time_offset_drops_are_recorded_loudly. That covers the previously-uncovered 575-576/611 lines, and the new _two_shard fixture (used by the aoi tests below) exercises the cross-shard join with both shards populated.

🤖 from Claude (review)

The time_start fallback admits raster entries that the raster dispatch path then refuses, so the loud refusal lands at run time instead of build time.

STAC allows (in fact requires) datetime: null when start_datetime/end_datetime are set, and sources.granule_records mirrors that: rec["datetime"] is only set when the datetime column is non-null (sources.py:869), while time_start/time_end are set independently (sources.py:852). Such a record reaches the shard map with assets + time_start and no datetime — this function pairs it happily, and then runner._raster_windowed_units blows up (runner.py:2375: raise ValueError(f"raster granule entry {e.get(\"id\")!r} carries no datetime")); the flat path is no better, runner.py:1455 builds keys = {None} and indexes time_index[None].

Probe (the phase-3 fixture with datetime nulled and start_datetime/end_datetime set):

record keys: ["assets", "https", "id", "lats", "lons", "s3", "time_end", "time_start"]
PAIRED OK. entry: {"id": "S2_0", ..., "time_start": "2019-05-06T22:50:22+00:00", "paired_epochs": [...]}
DISPATCH FAIL: ValueError raster granule entry "S2_0" carries no datetime

So the emitted map JSON round-trips, passes total_pairs bookkeeping, and dies on dispatch — the opposite of "dispatch consumes it unchanged".

Fix: backfill the instant the pairing actually used onto the emitted entry, so the map is self-consistent — in the chosen.setdefault construction add "datetime": entry.get("datetime") or entry.get("time_start") (the fallback value is already the one _acquisition_times keyed on). Failing that, refuse here for assets-carrying entries with no datetime, rather than deferring to the runner. Either way add a test: the fallback branch has no coverage today — test_a_granule_without_acquisition_time_refuses exercises only the both-missing case.

🤖 from Claude

Fixed in 23e391f — the backfill, not the refusal.

The chosen.setdefault construction now emits "datetime": src.get("datetime") or src.get("time_start"), with a comment naming the runner contract (runner._raster_windowed_units keys off datetime). It is the same instant _acquisition_times paired on, so the map dispatches exactly as it was built; _acquisition_times has already refused the both-missing case by the time this runs, so the value is never None. The closest_obs_shardmap Notes say so.

Refusing instead would have been the wrong half: a null-datetime + start_datetime record is valid STAC and carries a perfectly good instant — the map just was not writing it down.

Test test_a_start_datetime_only_record_still_emits_a_datetime builds the fixture with properties["datetime"] = None plus start_datetime/end_datetime, asserts the source record really lacks datetime (so the branch is exercised, not incidentally satisfied), and then that the emitted entry's datetime equals its time_start:

rec keys: ['assets', 'https', 'id', 'lats', 'lons', 's3', 'time_end', 'time_start']
entry:    {'id': 'S2_range', 'time_start': '2019-05-06T20:26:22+00:00', 'datetime': '2019-05-06T20:26:22+00:00'}

🤖 from Claude (review)

aoi is applied to one side of the join only, and this diagnostic pays for it. ref.epochs is already AOI-filtered (reference_epochs(..., aoi=aoi)), while ShardMap.build at line 563 runs with no region=/aoi= at all — so k not in ref.epochs counts AOI-excluded shards as "spatial shards without epochs", i.e. it reports "the catalog covers ground the reference stores never observed" when the truth is "I clipped it myself".

Probe (two shards in one catalog bbox, the cover claims both, aoi restricted to shard 11213):

spatial shards: ["11212", "11213"]
aoi=A only -> map shards: ["11213"]
  spatial_shards_without_epochs: 1   # 11212 HAS epochs in the store; the aoi excluded it

Two consequences worth separating:

1. The metric is wrong whenever aoi is passed, and it is the only signal an operator has for "my raster query and my reference coverage disagree". Fix: count against the unfiltered shard set — either compute it before the AOI clip (have reference_epochs also return the pre-clip shard set, or re-derive _aoi_shard_set(aoi, ref.order) here and exclude those keys), or omit the key entirely when aoi is not None rather than emit a number that means something else.
2. The spatial build is unrestricted: build(..., region=parts) exists and would cut the intersection down to the AOI. Today an AOI-scoped run still intersects every record in the catalog bbox (metadata["total_granules"] = 4 in the probe above, i.e. both shards) and discards the outside-AOI shards afterward. Correct, but at CONUS S2 scale that is the dominant cost, paid twice if the operator sizes the run with estimate=True first (the estimate path runs the same full ShardMap.build). Forwarding region= when aoi is ring parts / a GeoJSON path (and documenting the Moc case as epoch-side only) would fix it; if that is deliberate, the aoi parameter doc — "Restrict the shard set" — should say it restricts the epoch side only and the raster intersection still covers the whole catalog bbox.

🤖 from Claude

Both halves fixed in 2d72afa.

(1) The metric. The builder resolves the aoi once and derives aoi_keys = _aoi_shard_set(aoi, ref.order); spatial_shards_without_epochs now counts only spatial shards that lack epochs and sit inside the aoi, so an AOI-excluded shard is no longer reported as coverage disagreement.

(2) The spatial build. A GeoJSON path is resolved with load_polygon up front, and ring parts / that resolved polygon ride as ShardMap.build(..., region=parts). A mortie.Moc cannot become parts, so it keeps region=None and restricts the epoch side only — the aoi parameter doc now says exactly that. aoi=None is unchanged (region=None, the bbox-rectangle default).

Measured on the two-shard fixture (both shards in one catalog bbox, both covered):

unscoped build: shards ['11212', '11213']  total_pairs 4
region=aoi:     shards ['11213']            total_pairs 2

Tests (all on the new _two_shard fixture, with a ShardMap.build spy recording the kwargs):

• test_an_aoi_excluded_shard_is_not_coverage_disagreement — your 11212/11213 probe with a Moc aoi, which deliberately leaves the spatial build unscoped so 11212 is still assigned: region is None, map shards ["11213"], spatial_shards_without_epochs == 0. Reverting just the metric line makes it fail at the old value 1.
• test_ring_parts_aoi_scopes_the_spatial_build — region is None for aoi=None, region is AOI_AT_SHARD for parts, and the scoped map's pairs equal the unscoped map's pairs intersected with the aoi (cost changes, answer does not).
• test_a_geojson_aoi_scopes_the_spatial_build_as_parts — the path reaches build as ring parts, never as a str.

One thing left standing: metadata["total_granules"] still reads 4 under the scoped build, because build counts records considered (the bbox screen), not intersections. That is ShardMap.build's own accounting, unchanged by this PR.

🤖 from Claude (review)

"reconstructable from the manifest alone" holds only until someone calls a standard ShardMap method. reproject rebuilds every entry through _granule_entry (shardmap.py:1645, 1663, 1716), whose passthrough list is ("assets", "datetime", "time_key", "time_start", "time_end") — paired_epochs and epoch_offsets_ns are dropped, while metadata["closest_obs"] rides through untouched and keeps claiming them.

Probe (build, then reproject the same map):

built entry keys:   ["assets", "datetime", "epoch_offsets_ns", "https", "id", "paired_epochs", "s3"]
noop entry keys:    ["assets", "datetime", "https", "id", "s3"] | closest_obs still in meta: True | epochs_paired: 3
coarsen entry keys: ["assets", "datetime", "https", "id", "s3"] | closest_obs still in meta: True | epochs_paired: 3

Even the noop branch (target_order == source_order) strips it, so the loss is not confined to an order change. And on coarsen the surviving closest_obs block is worse than stale: dropped[*]["shard"], shards_without_acquisitions and coarsened_orders are keyed at the source order while the shards have been merged, so the manifest reads as a provenance record for a shard set that no longer exists.

This is a silent, undocumented loss of the one thing the Notes section promises. Minimal honest fix: add "paired_epochs" and "epoch_offsets_ns" to _granule_entry's key tuple (covers noop and coarsen; refine also needs them carried on sub_records beside the existing assets carry-through at shardmap.py:1691, and coarsen's dedupe-by-id would need to concatenate the two lists rather than keep one child's). If carrying provenance through reproject is out of scope for #509, then say so in the Notes here — "a reprojected map loses the provenance keys; do not derive from a paired map" — and have reproject pop closest_obs the way it already pops aoi_mask/build_wall_s, so a derived map does not advertise a pairing it no longer carries.

🤖 from Claude

Documented and pinned in ada8a48; the durable fix is left standing.

closest_obs_shardmap's Notes now carry the warning explicitly: do NOT derive a paired map with ShardMap.reproject — _granule_entry's passthrough does not know paired_epochs/epoch_offsets_ns, so a reprojected map (the noop same-order branch included) drops the provenance while metadata["closest_obs"] rides through describing the SOURCE map, and after a coarsen its shard ids name a shard set that no longer exists. Rebuild at the target grid instead.

test_reprojecting_a_paired_map_drops_the_provenance pins that, reproducing your probe (built entry has paired_epochs; the noop entry does not, and closest_obs is byte-identical to the source's). It is a wart-pinning test on purpose: whenever the two keys start surviving reproject, it fails and the Notes get updated with it.

Why not the real fix here: both durable options — adding the two keys to _granule_entry's tuple (plus the sub_records carry at shardmap.py:1691 and concatenating on coarsen's dedupe-by-id), or popping closest_obs in reproject beside aoi_mask/build_wall_s — live in shardmap.py, which is at 1,846 lines and which this PR must not grow. Leaving that half for review.

On a zero-shardmap-edit runtime guard, for consideration rather than as something I implemented: ShardMap is a plain dataclass, so a paired map could be returned as a thin ClosestObsShardMap subclass whose reproject refuses (or drops closest_obs from the derived map's metadata) — no shardmap.py edit, and from_json/from_parquet would still hand back a plain ShardMap, so it only guards the in-process path, not a manifest reloaded from disk. That partial coverage is why I did not reach for it unprompted; happy to add it if it is wanted.

🤖 from Claude (review)

assert ids is vacuous — it only says "something was selected", so it cannot fail for the reason the comment above it gives ("the near-gap acquisitions ARE selected (the epochs at the gap's shoulders reach them)").

Probe against this exact fixture (uv run python, TestTwoStoreScenarios._stores + _catalog, max_time_offset=3 D):

epoch days: [-0.047, 1.989, 4.025, 6.062, 8.098, 10.134, 50.042, 52.078, 54.114, 54.929, 56.150, 58.187, 59.815]
selected  : S2_0 S2_1 S2_2 S2_12 S2_13
  S2_2  (day  8.6) <- epochs at days  8.098, 10.134   # left shoulder
  S2_12 (day 51.6) <- epochs at days 50.042, 52.078   # right shoulder

so the shoulder claim is true but unpinned: assert ids still passes if the shoulder epochs stop reaching (e.g. it passes unchanged under max_time_offset=1 D, where four of the five right-hand epochs drop, because S2_0 alone keeps ids non-empty). The first assertion (gap and not (ids & gap)) also has ~7 days of margin (nearest gap acquisition is 7.04 d from any epoch, vs a 3 d cap), so nothing in this test pins the shoulder reach that makes the gap result meaningful.

Fix: replace assert ids with the thing the comment claims —

assert {"S2_2", "S2_12"} <= ids  # the gap's shoulder epochs reach their neighbours

(or pin the whole set, ids == {"S2_0", "S2_1", "S2_2", "S2_12", "S2_13"}, which subsumes the gap assertion too).

🤖 from Claude

Fixed in e9ac3c8. assert ids is now assert {"S2_2", "S2_12"} <= ids, with the comment pinning the reach it stands for (S2_2 ← epochs at days 8.098/10.134, S2_12 ← 50.042/52.078). The gap-exclusion assertion above it is unchanged.

Confirmed the new assertion is actually falsifiable where the old one was not — sweeping max_time_offset on the same fixture:

24h  ids=[S2_0,S2_1,S2_2,S2_12,S2_13]  old_pass=True  new_pass=True
12h  ids=[S2_0,S2_1,S2_12,S2_13]       old_pass=True  new_pass=False
 8h  ids=[S2_0,S2_1,S2_13]             old_pass=True  new_pass=False
 6h  ids=[S2_0]                        old_pass=True  new_pass=False

Kept the subset form rather than pinning the whole set, because the fixture change folded from the next thread adds S2_3 to this shard's selection; <= survives that, == would not.

🤖 from Claude (review)

The union-parity assertion is satisfied by a builder that ignores store A entirely, because this fixture makes A's selection a subset of B's.

Probe (same fixture, max_time_offset=3 D, pairs as (shard, id) sets):

A (atl03, days 0,55)                : ['S2_0', 'S2_13']
B (gedi,  days 0..10,50..60)        : ['S2_0', 'S2_1', 'S2_2', 'S2_12', 'S2_13']
A <= B ?  True
A|B == B  ->  pairs(map([a,b])) == pairs(map(b)) == pairs(map(a)) | pairs(map(b))

So map([a, b]) == map(a) | map(b) holds identically for map([a,b]) := map(b) — i.e. a regression that dropped every reference store but the last (or that unioned epochs on reference_stores[-1] only) passes this test green. A's two epochs (days 0 and 55) select granules B already selects, so A contributes nothing unique to the union.

Note the word-level-vs-bucket-level union bug the module docstring warns about is not reachable here either — every word in both covers spans exactly one bucket (probed via cover_words + toc2time: buckets/word == [1]*2 for A, [1]*12 for B), so word-union and bucket-union coincide. That class is pinned by test_partially_overlapping_covers_union_at_the_bucket_grid at the epoch level, so the builder-level test's only distinct job is exactly the per-store-contribution one it currently cannot fail.

Fix: give A one pass no B epoch reaches, e.g. A_DAYS = (0, 13, 55) (line 828). Probed with that single change:

A: ['S2_0', 'S2_3', 'S2_13']   A <= B ? False   AB: [S2_0,S2_1,S2_2,S2_3,S2_12,S2_13]   parity: True
gap test still clean: no (13,47)-day acquisition selected  (day-13 epoch -> S2_3 at 12.9 d; 17.2 d is 4.2 d away, past the 3 d cap)

Optionally add assert _pairs(map(a)) < _pairs(map([a, b])) so the asymmetry stays pinned if the fixture drifts again.

🤖 from Claude

Fixed in 1228b3e. A_DAYS is now (0, 13, 55), with a comment saying what day 13 is load-bearing for. Re-probed on the changed fixture:

A : [S2_0, S2_3, S2_13]      A <= B ? False
B : [S2_0, S2_1, S2_2, S2_12, S2_13]
AB: [S2_0, S2_1, S2_2, S2_3, S2_12, S2_13]     parity: True
gap test: still clean (no (13,47)-day acquisition selected); gap set unchanged
          [S2_4..S2_10]; shoulders {S2_2, S2_12} still selected

So the gap test needed no adjustment to its pinned set — S2_3 sits at day 12.9, outside 13 < d < 47, and the first gap acquisition at 17.2 d is 4.2 d from the day-13 epoch, past the 3 d cap.

Took the optional suggestion too, symmetrically: the test now binds the three maps and asserts only_a < both and only_b < both beside the parity equality, so dropping either store fails rather than only the last-wins case.

Also strengthened test_a_covered_shard_with_no_acquisitions_is_recorded per the non-filed note in the same commit — it now asserts the ledgered rows for shard 11212 rather than only its membership in shards_without_acquisitions: exactly two rows, every nearest_offset_ns None, and the two epochs within HALF_BUCKET of the store's day-1/day-3 instants (they are bucket midpoints, not the raw instants, so the ±4.9 h bound is the right pin here).

Full suite green: tests/test_closest_obs.py tests/test_coverage_toc.py tests/test_shardmap.py → 315 passed, 1 skipped.

🤖 from Claude (review)

The dry-run example cannot report what the prose two paragraphs down promises it reports: "a shard past max_granules_per_shard raises naming the worst shards (estimate=True reports the violations instead, so the gate can be sized first)". This estimate=True call omits max_granules_per_shard, and violations is computed from that argument — [] whenever it is None.

Probe (uv run python, synthetic two-store fixture, one shard with 5 selected granules):

closest_obs_shardmap(..., estimate=True)                            -> violations []
closest_obs_shardmap(..., estimate=True, max_granules_per_shard=3)  -> violations [('11213', 5)]
closest_obs_shardmap(..., max_granules_per_shard=3)                 -> ValueError: 1 shard(s) exceed max_granules_per_shard=3 (worst: 11213=5)

So a reader who copies this block verbatim to "size the run first" gets a silent violations == [], then hits the raise on the very next call — the failure the dry run exists to prevent. (estimate returns before the violations raise in closest_obs_shardmap, so passing the gate here is safe and non-raising.)

Fix: pass the same gate in the dry run as in the build, and surface it:

    max_time_offset=np.timedelta64(3, "D"),
    max_granules_per_shard=200,
    estimate=True,
)
est["histogram"], est["max_cost_usd"], est["violations"]

🤖 from Claude

Fixed in bb870dc. The dry-run call now passes max_granules_per_shard=200 — the same gate the build call below it uses — and the trailing line reads est["histogram"], est["max_cost_usd"], est["violations"], with a short comment saying why the gate has to be repeated here and that estimate returns before the raise.

Re-probed the behaviour the fix depends on, on a synthetic two-store fixture:

estimate=True                            -> violations []
estimate=True, max_granules_per_shard=3  -> violations [("11213", 6)]
              max_granules_per_shard=3   -> ValueError: 1 shard(s) exceed max_granules_per_shard=3 (worst: 11213=6)

so the gated dry run is non-raising and reports what the prose promises. mkdocs build --strict is green.

🤖 from Claude (review)

The resolution gate sits after the i is None (no-acquisitions) branch at line 656, so a coarse epoch in a shard the catalog never reaches is ledgered as a no-acquisition row and epochs_dropped_low_resolution silently undercounts — which contradicts the unconditional claim this commit adds to the docstring (line 541-ff: "is dropped into the ledger as its own category ... counted in epochs_dropped_low_resolution") and to docs/api/catalog.md.

Probe (pristine worktree, order-12 cover on shard 11213, cap 3 d, catalog footprint moved to lat -40/lon -120 so the shard has no spatially-assigned acquisitions):

epochs_total 1
epochs_paired 0
epochs_dropped 1
epochs_dropped_low_resolution 0      <- the ruling's category, empty
shards_without_acquisitions ['11213']
coarsened_orders {'11213': 12}
dropped[0]: {'shard': '11213', 'epoch': '2019-06-09T09:37:19.595621375', 'nearest_offset_ns': None}

Why it matters rather than being cosmetic: unresolvability is a property of the epoch and its cover block alone — it does not depend on whether the raster catalog happens to reach that shard — yet the category the operator reads flips on that unrelated state. And the existing comment two lines above says a catalog-gap shard "is the largest drop class in practice", so this is the collision that will actually happen at scale: an operator sizing max_time_offset off epochs_dropped_low_resolution == 0 concludes the cap is fine, fixes the catalog gap, and the same epochs then reappear as resolution drops.

Concrete fix — hoist the gate above the spatial lookup, and keep the catalog-gap signal by moving the empty-array continue below the i is None branch:

decimal = morton_decimal(shard)
eo = ref.epoch_orders.get(shard)
if cap_ns is not None and eo is not None:
    ...ledger the unresolvable rows, epoch_arr = epoch_arr[~unresolvable]...
i = spatial_idx.get(shard)
if i is None:
    no_acquisitions.append(decimal)
    dropped.extend(...for t in epoch_arr)   # only the SURVIVING epochs
    continue
if epoch_arr.size == 0:
    continue
entries = spatial.granules[i]

That keeps shards_without_acquisitions naming the shard (the row is about the shard, not the epochs), keeps the epochs_total == epochs_paired + epochs_dropped invariant, and puts each epoch in exactly one category. A test worth adding: the probe above, asserting epochs_dropped_low_resolution == epochs_total while shards_without_acquisitions == ["11213"].

🤖 from Claude

Fixed in 073242e. The gate is hoisted above the spatial lookup exactly as proposed: the eo/half_span/unresolvable block now runs first, i = spatial_idx.get(shard) follows, the i is None branch appends the shard to no_acquisitions and ledgers only the SURVIVING epochs as no-acquisition rows, and the empty-epoch_arr continue moved below that branch.

Probe, same setup as yours (order-12 cover on 11213, cap 3 d, catalog footprint at lat -40/lon -120), now:

epochs_total 1
epochs_paired 0
epochs_dropped 1
epochs_dropped_low_resolution 1      <- the ruling's category, populated
shards_without_acquisitions ['11213']
dropped[0]: {'shard': '11213', 'epoch': ..., 'temporal_order': 12, 'cover_half_span_ns': 1125899906842624}

Two tests, both failing before the commit and passing after:

• test_an_unreached_shard_still_counts_its_resolution_drops — your probe: epochs_dropped_low_resolution == epochs_total, every dropped row carries temporal_order, shards_without_acquisitions == ["11213"], invariant holds.
• test_an_unreached_shard_splits_coarse_from_surviving_epochs — a pinned store beside the coarsened one in the same unreached shard: the coarse epochs count as resolution drops, the survivors become no-acquisition rows (nearest_offset_ns is None), the shard is still named, invariant holds. Each epoch lands in exactly one class.

The comment in the i is None branch now says the shard row is about the SHARD, not its epochs, and the gate carries a note on why it sits ahead of the lookup.

🤖 from Claude (review)

Loudness is inverted between the two arms of the ruling. The no-cap arm — where nothing is discarded and the build proceeds — gets this purpose-built warning naming the effective resolution. The with-cap arm — where epochs are actually thrown away — gets no dedicated warning at all; the only builder log it reaches is the generic summary at line 736, whose parenthetical enumerates two causes and neither of them is the real one.

Probe (pristine worktree, order-12 block, max_time_offset=3 D, catalog reaching the shard) — every warning the build emits:

reference_epochs: store ... shard 11213 cover sits at temporal order 12, below the pinned 18 ...
closest_obs_shardmap: 1 epoch(s) selected nothing (max_time_offset=np.timedelta64(3,'D'),
  or no acquisitions in the shard); e.g. [{'shard': '11213', ..., 'temporal_order': 12,
  'cover_half_span_ns': 1125899906842624}] ...

epochs_dropped_low_resolution == 1, and no emitted line contains the word "resolution". The epoch did not "select nothing" because of max_time_offset=3 D (no selection ever ran for it) and not because the shard has no acquisitions (it has one) — the message states two causes, both wrong for this row. The reference_epochs coarsening warning fires, but it says only "these epochs are good to ±2^50 ns"; it never says they were consequently dropped, and it fires identically when no cap is set and nothing is dropped.

Concrete fix — emit the with-cap counterpart right beside this block, so the arm that discards data is at least as loud as the arm that does not:

if low_resolution:
    logger.warning(
        f"closest_obs_shardmap: {low_resolution} epoch(s) dropped as UNRESOLVABLE at "
        f"max_time_offset={max_time_offset!r} — their cover blocks' bucket half-span "
        f"exceeds it (coarsened orders {coarse}); recorded in "
        f"metadata['closest_obs']['dropped'] with temporal_order/cover_half_span_ns"
    )

and, minimally, add the third cause to line 736's parenthetical ("...or a cover block too coarse for the offset") so the summary stops mis-attributing those rows. Nothing in TestCoarsenedCoverTolerance asserts on the log for the with-cap arm, while test_no_cap_warns_once_and_pairs_everything does for the other — a symmetric caplog assertion would pin it.

🤖 from Claude

Fixed in ae30bab. Three parts:

1. A purpose-built if low_resolution: warning beside the summary, so the arm that discards data is at least as loud as the arm that does not. It names the count, the cap, and the cause explicitly as not distance:

closest_obs_shardmap: 1 epoch(s) dropped as UNRESOLVABLE at max_time_offset=np.timedelta64(3,'D') —
a coarsened cover, NOT distance to an acquisition: their block's bucket half-span exceeds the
offset (coarsest temporal order 12 across 1 shard(s), e.g. ['11213']) — every row is in
metadata['closest_obs']['dropped'] with temporal_order/cover_half_span_ns, counted in
metadata['closest_obs']['epochs_dropped_low_resolution']

2. The generic summary's parenthetical gained the third cause — now (max_time_offset=..., no acquisitions in the shard, or a cover block too coarse for the offset) — so it stops mis-attributing those rows. coarse moved above the warning block to feed both.

3. test_the_cap_arm_warns_that_a_coarsened_cover_dropped_them, symmetric to test_no_cap_warns_once_and_pairs_everything: exactly one UNRESOLVABLE line, carrying the count, "a coarsened cover, NOT distance", and temporal order 12; the summary carries the third cause; and the no-cap line does not fire under a cap (the two arms stay distinguishable in the log).

🤖 from Claude (review)

Low severity, but it is a fail-open on the new precision bar, so worth a line. The int64 shift is safe on the upper side — _decode_cover_block MUST-checks order > pinned and refuses, and the pin is 18, so order ≥ 63 is unreachable. But that guard is one-sided: there is no lower bound, so a corrupt/hand-written block declaring a negative temporal_order decodes fine, and the shift then overflows instead of raising.

Probe:

order   0: half_span 4611686018427387904  >cap(1e9)? True
order  18: half_span 17592186044416       >cap(1e9)? True
order  62: half_span 1                    >cap(1e9)? False
order  -1: half_span -9223372036854775808 >cap(1e9)? False   <- passes the bar

and upstream _word_midpoints does not refuse it either (order=-1 returns a midpoint of 2142-04-11T23:46:54, from words >> uint64(64)). Net effect: a store whose cover block claims order -1 produces garbage epochs that then sail through a precision bar the caller explicitly set — the one thing this commit exists to prevent. numpy emits no warning; the wraparound is silent.

Cheapest fix is upstream and one line, in coverage_toc._decode_cover_block beside the existing ceiling check:

if order < 0 or order > int(pinned):
    raise ValueError(... "a block only ever coarsens BELOW the pin, and never below 0 (spec §10.5)")

which makes both this gate and _word_midpoints' k = 63 - order total over the accepted domain. If touching coverage_toc.py is out of this PR's scope, declining with a filed follow-up is defensible — but an in-file np.clip(eo, 0, 62) would only mask it, so please don't take that route.

🤖 from Claude

Fixed in 7df30da, on this module's side, as a loud boundary refusal — not a clip.

Reproduced first, pristine: a block with temporal_order: -1 decoded fine and reference_epochs returned orders {…: -1}, an epoch of 2142-04-11T23:46:54, and half_span -9223372036854775808 — through a 3 d cap, silently.

The fix sits in reference_epochs' per-block loop, where the order is read off the grammar:

raw = block.get("temporal_order", pinned)
if isinstance(raw, bool) or not isinstance(raw, int) or raw < 0:
    raise ValueError(
        f"reference_epochs: store {root!r} shard {decimal} cover block declares "
        f"temporal_order {raw!r} — a block only ever coarsens BELOW the object's "
        f"pin and never below 0 (spec §10.5); this block is corrupt, and decoding "
        f"it would yield epochs no precision bar can hold"
    )

It names the store, the shard decimal, and the corrupt order, and cites §10.5. No np.clip — agreed that would mask it.

To be explicit about scope: the durable one-line guard belongs in coverage_toc._decode_cover_block beside the existing ceiling check (if order < 0 or order > int(pinned)), which is what makes both this gate and _word_midpoints' k = 63 - order total over the accepted domain for every caller, not just this builder. coverage_toc.py is read-only for this PR (it sits near the module cap), so that half is left standing for espg — same handling as the reproject-provenance finding's shardmap-side half.

Test: test_a_negative_block_order_refuses_by_name pins the refusal through both entry points — reference_epochs directly and closest_obs_shardmap — with pytest.raises(ValueError, match="temporal_order -1").